From f207095aac35b93267aaf09faee33cfe80f04f5e Mon Sep 17 00:00:00 2001 From: "emellor@leeni.uk.xensource.com" Date: Fri, 31 Mar 2006 00:24:54 +0100 Subject: [PATCH] Remove the transaction parameter from xenbus_switch_state and move the state switch out of a transaction, in the few cases where it is inside one. In order to behave properly, it is necessary for a driver to know its own xenbus state (see changeset 9469:b3cb19d2b07f, for example). This value is stored as xenbus_device.state and updated by xenbus_switch_state. If xenbus_switch_state occurs within a transaction, then there is a possibility that the transaction would be aborted, leaving the state field dangerously out of sync with the value currently in the store. This fixes recent problems seen whereby bringing multiple devices up at the same time results in some devices not coming up (often all of the even-numbered ones, because of the pattern of transaction conflict). Signed-off-by: Ewan Mellor --- .../drivers/xen/blkback/xenbus.c | 14 ++++++++------ .../drivers/xen/blkfront/blkfront.c | 12 +++++------- .../drivers/xen/netback/xenbus.c | 6 +++--- .../drivers/xen/netfront/netfront.c | 2 +- .../drivers/xen/pciback/xenbus.c | 8 ++++---- .../drivers/xen/pcifront/xenbus.c | 11 ++++------- .../drivers/xen/tpmback/xenbus.c | 17 +++++++---------- .../drivers/xen/xenbus/xenbus_client.c | 18 +++++++++++------- .../drivers/xen/xenbus/xenbus_probe.c | 4 ++-- linux-2.6-xen-sparse/include/xen/xenbus.h | 10 +++------- 10 files changed, 48 insertions(+), 54 deletions(-) diff --git a/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c b/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c index 23fe51a362..636caefdd8 100644 --- a/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c +++ b/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c @@ -142,7 +142,7 @@ static int blkback_probe(struct xenbus_device *dev, if (err) goto fail; - err = xenbus_switch_state(dev, XBT_NULL, XenbusStateInitWait); + err = xenbus_switch_state(dev, XenbusStateInitWait); if (err) goto fail; @@ -270,7 +270,7 @@ static void frontend_changed(struct xenbus_device *dev, break; case XenbusStateClosing: - xenbus_switch_state(dev, XBT_NULL, XenbusStateClosing); + xenbus_switch_state(dev, XenbusStateClosing); break; case XenbusStateClosed: @@ -343,15 +343,17 @@ again: goto abort; } - err = xenbus_switch_state(dev, xbt, XenbusStateConnected); - if (err) - goto abort; - err = xenbus_transaction_end(xbt, 0); if (err == -EAGAIN) goto again; if (err) xenbus_dev_fatal(dev, err, "ending transaction"); + + err = xenbus_switch_state(dev, XenbusStateConnected); + if (err) + xenbus_dev_fatal(dev, err, "switching to Connected state", + dev->nodename); + return; abort: xenbus_transaction_end(xbt, 1); diff --git a/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c b/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c index 6e29fa3d75..4999636207 100644 --- a/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c +++ b/linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c @@ -176,10 +176,6 @@ again: goto abort_transaction; } - err = xenbus_switch_state(dev, xbt, XenbusStateInitialised); - if (err) - goto abort_transaction; - err = xenbus_transaction_end(xbt, 0); if (err) { if (err == -EAGAIN) @@ -188,6 +184,8 @@ again: goto destroy_blkring; } + xenbus_switch_state(dev, XenbusStateInitialised); + return 0; abort_transaction: @@ -324,7 +322,7 @@ static void connect(struct blkfront_info *info) return; } - (void)xenbus_switch_state(info->xbdev, XBT_NULL, XenbusStateConnected); + (void)xenbus_switch_state(info->xbdev, XenbusStateConnected); /* Kick pending requests. */ spin_lock_irq(&blkif_io_lock); @@ -349,7 +347,7 @@ static void blkfront_closing(struct xenbus_device *dev) xlvbd_del(info); - xenbus_switch_state(dev, XBT_NULL, XenbusStateClosed); + xenbus_switch_state(dev, XenbusStateClosed); } @@ -755,7 +753,7 @@ static void blkif_recover(struct blkfront_info *info) kfree(copy); - (void)xenbus_switch_state(info->xbdev, XBT_NULL, XenbusStateConnected); + (void)xenbus_switch_state(info->xbdev, XenbusStateConnected); /* Now safe for us to use the shared ring */ spin_lock_irq(&blkif_io_lock); diff --git a/linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c b/linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c index 52a4fc98c8..ea9a02e28e 100644 --- a/linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c +++ b/linux-2.6-xen-sparse/drivers/xen/netback/xenbus.c @@ -92,7 +92,7 @@ static int netback_probe(struct xenbus_device *dev, if (err) goto fail; - err = xenbus_switch_state(dev, XBT_NULL, XenbusStateInitWait); + err = xenbus_switch_state(dev, XenbusStateInitWait); if (err) { goto fail; } @@ -209,7 +209,7 @@ static void frontend_changed(struct xenbus_device *dev, break; case XenbusStateClosing: - xenbus_switch_state(dev, XBT_NULL, XenbusStateClosing); + xenbus_switch_state(dev, XenbusStateClosing); break; case XenbusStateClosed: @@ -254,7 +254,7 @@ static void connect(struct backend_info *be) return; } - xenbus_switch_state(dev, XBT_NULL, XenbusStateConnected); + xenbus_switch_state(dev, XenbusStateConnected); } diff --git a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c index 220e9aa343..6c7b82f789 100644 --- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c +++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c @@ -1216,7 +1216,7 @@ static void netfront_closing(struct xenbus_device *dev) close_netdev(info); - xenbus_switch_state(dev, XBT_NULL, XenbusStateClosed); + xenbus_switch_state(dev, XenbusStateClosed); } diff --git a/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c b/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c index c64d250067..dc369d0efb 100644 --- a/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c +++ b/linux-2.6-xen-sparse/drivers/xen/pciback/xenbus.c @@ -137,7 +137,7 @@ static int pciback_attach(struct pciback_device *pdev) dev_dbg(&pdev->xdev->dev, "Connecting...\n"); - err = xenbus_switch_state(pdev->xdev, XBT_NULL, XenbusStateConnected); + err = xenbus_switch_state(pdev->xdev, XenbusStateConnected); if (err) xenbus_dev_fatal(pdev->xdev, err, "Error switching to connected state!"); @@ -165,7 +165,7 @@ static void pciback_frontend_changed(struct xenbus_device *xdev, break; case XenbusStateClosing: - xenbus_switch_state(xdev, XBT_NULL, XenbusStateClosing); + xenbus_switch_state(xdev, XenbusStateClosing); break; case XenbusStateClosed: @@ -341,7 +341,7 @@ static int pciback_setup_backend(struct pciback_device *pdev) goto out; } - err = xenbus_switch_state(pdev->xdev, XBT_NULL, XenbusStateInitialised); + err = xenbus_switch_state(pdev->xdev, XenbusStateInitialised); if (err) xenbus_dev_fatal(pdev->xdev, err, "Error switching to initialised state!"); @@ -386,7 +386,7 @@ static int pciback_xenbus_probe(struct xenbus_device *dev, } /* wait for xend to configure us */ - err = xenbus_switch_state(dev, XBT_NULL, XenbusStateInitWait); + err = xenbus_switch_state(dev, XenbusStateInitWait); if (err) goto out; diff --git a/linux-2.6-xen-sparse/drivers/xen/pcifront/xenbus.c b/linux-2.6-xen-sparse/drivers/xen/pcifront/xenbus.c index eed89eb5d3..ee154ba360 100644 --- a/linux-2.6-xen-sparse/drivers/xen/pcifront/xenbus.c +++ b/linux-2.6-xen-sparse/drivers/xen/pcifront/xenbus.c @@ -96,10 +96,6 @@ static int pcifront_publish_info(struct pcifront_device *pdev) if (!err) err = xenbus_printf(trans, pdev->xdev->nodename, "magic", XEN_PCI_MAGIC); - if (!err) - err = - xenbus_switch_state(pdev->xdev, trans, - XenbusStateInitialised); if (err) { xenbus_transaction_end(trans, 1); @@ -118,6 +114,8 @@ static int pcifront_publish_info(struct pcifront_device *pdev) } } + xenbus_switch_state(pdev->xdev, XenbusStateInitialised); + dev_dbg(&pdev->xdev->dev, "publishing successful!\n"); out: @@ -186,7 +184,7 @@ static int pcifront_try_connect(struct pcifront_device *pdev) } } - err = xenbus_switch_state(pdev->xdev, XBT_NULL, XenbusStateConnected); + err = xenbus_switch_state(pdev->xdev, XenbusStateConnected); if (err) goto out; @@ -205,8 +203,7 @@ static int pcifront_try_disconnect(struct pcifront_device *pdev) prev_state = xenbus_read_driver_state(pdev->xdev->nodename); if (prev_state < XenbusStateClosing) - err = xenbus_switch_state(pdev->xdev, XBT_NULL, - XenbusStateClosing); + err = xenbus_switch_state(pdev->xdev, XenbusStateClosing); if (!err && prev_state == XenbusStateConnected) pcifront_disconnect(pdev); diff --git a/linux-2.6-xen-sparse/drivers/xen/tpmback/xenbus.c b/linux-2.6-xen-sparse/drivers/xen/tpmback/xenbus.c index 6ce5f07891..14d6feb75e 100644 --- a/linux-2.6-xen-sparse/drivers/xen/tpmback/xenbus.c +++ b/linux-2.6-xen-sparse/drivers/xen/tpmback/xenbus.c @@ -87,7 +87,7 @@ static int tpmback_probe(struct xenbus_device *dev, goto fail; } - err = xenbus_switch_state(dev, XBT_NULL, XenbusStateInitWait); + err = xenbus_switch_state(dev, XenbusStateInitWait); if (err) { goto fail; } @@ -175,7 +175,7 @@ static void frontend_changed(struct xenbus_device *dev, break; case XenbusStateClosing: - xenbus_switch_state(dev, XBT_NULL, XenbusStateClosing); + xenbus_switch_state(dev, XenbusStateClosing); break; case XenbusStateClosed: @@ -247,18 +247,15 @@ again: goto abort; } - err = xenbus_switch_state(dev, xbt, XenbusStateConnected); - if (err) - goto abort; - - be->tpmif->status = CONNECTED; - err = xenbus_transaction_end(xbt, 0); if (err == -EAGAIN) goto again; - if (err) { + if (err) xenbus_dev_fatal(be->dev, err, "end of transaction"); - } + + err = xenbus_switch_state(dev, XenbusStateConnected); + if (!err) + be->tpmif->status = CONNECTED; return; abort: xenbus_transaction_end(xbt, 1); diff --git a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c index 8126705aeb..38ac7f2c49 100644 --- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c +++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_client.c @@ -84,9 +84,7 @@ int xenbus_watch_path2(struct xenbus_device *dev, const char *path, EXPORT_SYMBOL_GPL(xenbus_watch_path2); -int xenbus_switch_state(struct xenbus_device *dev, - xenbus_transaction_t xbt, - XenbusState state) +int xenbus_switch_state(struct xenbus_device *dev, XenbusState state) { /* We check whether the state is currently set to the given value, and if not, then the state is set. We don't want to unconditionally @@ -94,6 +92,12 @@ int xenbus_switch_state(struct xenbus_device *dev, unnecessarily. Furthermore, if the node has gone, we don't write to it, as the device will be tearing down, and we don't want to resurrect that directory. + + Note that, because of this cached value of our state, this function + will not work inside a Xenstore transaction (something it was + trying to in the past) because dev->state would not get reset if + the transaction was aborted. + */ int current_state; @@ -102,12 +106,12 @@ int xenbus_switch_state(struct xenbus_device *dev, if (state == dev->state) return 0; - err = xenbus_scanf(xbt, dev->nodename, "state", "%d", - ¤t_state); + err = xenbus_scanf(XBT_NULL, dev->nodename, "state", "%d", + ¤t_state); if (err != 1) return 0; - err = xenbus_printf(xbt, dev->nodename, "state", "%d", state); + err = xenbus_printf(XBT_NULL, dev->nodename, "state", "%d", state); if (err) { if (state != XenbusStateClosing) /* Avoid looping */ xenbus_dev_fatal(dev, err, "writing new state"); @@ -193,7 +197,7 @@ void xenbus_dev_fatal(struct xenbus_device *dev, int err, const char *fmt, _dev_error(dev, err, fmt, ap); va_end(ap); - xenbus_switch_state(dev, XBT_NULL, XenbusStateClosing); + xenbus_switch_state(dev, XenbusStateClosing); } EXPORT_SYMBOL_GPL(xenbus_dev_fatal); diff --git a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c index 61158a6034..d3f37e636d 100644 --- a/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c +++ b/linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c @@ -364,7 +364,7 @@ static int xenbus_dev_probe(struct device *_dev) return 0; fail: xenbus_dev_error(dev, err, "xenbus_dev_probe on %s", dev->nodename); - xenbus_switch_state(dev, XBT_NULL, XenbusStateClosed); + xenbus_switch_state(dev, XenbusStateClosed); return -ENODEV; } @@ -381,7 +381,7 @@ static int xenbus_dev_remove(struct device *_dev) if (drv->remove) drv->remove(dev); - xenbus_switch_state(dev, XBT_NULL, XenbusStateClosed); + xenbus_switch_state(dev, XenbusStateClosed); return 0; } diff --git a/linux-2.6-xen-sparse/include/xen/xenbus.h b/linux-2.6-xen-sparse/include/xen/xenbus.h index 6d69963870..b5a9da5f64 100644 --- a/linux-2.6-xen-sparse/include/xen/xenbus.h +++ b/linux-2.6-xen-sparse/include/xen/xenbus.h @@ -204,14 +204,10 @@ int xenbus_watch_path2(struct xenbus_device *dev, const char *path, /** * Advertise in the store a change of the given driver to the given new_state. - * Perform the change inside the given transaction xbt. xbt may be NULL, in - * which case this is performed inside its own transaction. Return 0 on - * success, or -errno on error. On error, the device will switch to - * XenbusStateClosing, and the error will be saved in the store. + * Return 0 on success, or -errno on error. On error, the device will switch + * to XenbusStateClosing, and the error will be saved in the store. */ -int xenbus_switch_state(struct xenbus_device *dev, - xenbus_transaction_t xbt, - XenbusState new_state); +int xenbus_switch_state(struct xenbus_device *dev, XenbusState new_state); /** -- 2.30.2